Remove the IMF_READ_INTERRUPT_ID build option

The IMF_READ_INTERRUPT_ID build option enables a feature where the interrupt
ID of the highest priority pending interrupt is passed as a parameter to the
interrupt handler registered for that type of interrupt. This additional read
of highest pending interrupt id from GIC is problematic as it is possible that
the original interrupt may get deasserted and another interrupt of different
type maybe become the highest pending interrupt. Hence it is safer to prevent
such behaviour by removing the IMF_READ_INTERRUPT_ID build option.

The `id` parameter of the interrupt handler `interrupt_type_handler_t` is
now made a reserved parameter with this patch. It will always contain
INTR_ID_UNAVAILABLE.

Fixes ARM-software/tf-issues#307

Change-Id: I2173aae1dd37edad7ba6bdfb1a99868635fa34de

diff --git a/bl31/aarch64/runtime_exceptions.S b/bl31/aarch64/runtime_exceptions.S
index bbc7f1b46cbf04a266d1e6c1b20f42885fe19882..28353202dab650fa5a1028648c1af01c162d30bb 100644 (file)
--- a/bl31/aarch64/runtime_exceptions.S
+++ b/bl31/aarch64/runtime_exceptions.S
@@ -130,16 +130,6 @@
        mov     x21, x0
 
        mov     x0, #INTR_ID_UNAVAILABLE
-#if IMF_READ_INTERRUPT_ID
-       /*
-        * Read the id of the highest priority pending interrupt. If
-        * no interrupt is asserted then return to where we came from.
-        */
-       mov     x19,  #INTR_ID_UNAVAILABLE
-       bl      plat_ic_get_pending_interrupt_id
-       cmp     x19, x0
-       b.eq    interrupt_exit_\label
-#endif
 
        /* Set the current security state in the 'flags' parameter */
        mrs     x2, scr_el3

diff --git a/bl31/bl31.mk b/bl31/bl31.mk
index 04e154200fe021d59507a4127697c0f6456d4d48..a31c1f475f4b0961bbc378f91626a207bb7ee830 100644 (file)
--- a/bl31/bl31.mk
+++ b/bl31/bl31.mk
@@ -60,13 +60,6 @@ endif
 
 BL31_LINKERFILE                :=      bl31/bl31.ld.S
 
-# Flag used by the generic interrupt management framework to  determine if
-# upon the assertion of an interrupt, it should pass the interrupt id or not
-IMF_READ_INTERRUPT_ID  :=      0
-
-$(eval $(call assert_boolean,IMF_READ_INTERRUPT_ID))
-$(eval $(call add_define,IMF_READ_INTERRUPT_ID))
-
 # Flag used to inidicate if Crash reporting via console should be included
 # in BL3-1. This defaults to being present in DEBUG builds only
 ifndef CRASH_REPORTING

diff --git a/docs/interrupt-framework-design.md b/docs/interrupt-framework-design.md
index cee29a318c431270a46039e50e61e951ad9cc6d6..53707ae9d8259fdf080c1f7298c6f2129ea98e20 100644 (file)
--- a/docs/interrupt-framework-design.md
+++ b/docs/interrupt-framework-design.md
@@ -293,11 +293,9 @@ This component declares the following prototype for a handler of an interrupt ty
                                             void *handle,
                                             void *cookie);
 
-The value of the `id` parameter depends upon the definition of the
-`IMF_READ_INTERRUPT_ID` build time flag. When the flag is defined, `id` contains
-the number of the highest priority pending interrupt of the type that this
-handler was registered for. When the flag is not defined `id` contains
-`INTR_ID_UNAVAILABLE`.
+The `id` is parameter is reserved and could be used in the future for passing
+the interrupt id of the highest pending interrupt only if there is a foolproof
+way of determining the id. Currently it contains `INTR_ID_UNAVAILABLE`.
 
 The `flags` parameter contains miscellaneous information as follows.
 
@@ -583,11 +581,10 @@ responsible for:
     irrecoverable error condition.
 
 6.  Calling the registered handler function for the interrupt type generated.
-    The firmware also determines the interrupt id if the IMF_READ_INTERRUPT_ID
-    build time flag is set. The id is set to `INTR_ID_UNAVAILABLE` if the flag
-    is not set. The id along with the current security state and a reference to
-    the `cpu_context_t` structure for the current security state are passed to
-    the handler function as its arguments.
+    The `id` parameter is set to `INTR_ID_UNAVAILABLE` currently. The id along
+    with the current security state and a reference to the `cpu_context_t`
+    structure for the current security state are passed to the handler function
+    as its arguments.
 
     The handler function returns a reference to the per-cpu `cpu_context_t`
     structure for the target security state.

diff --git a/docs/porting-guide.md b/docs/porting-guide.md
index 6e71025cc6815cd7bf239a13ab393331186e1e04..8c0d7b7cca6ebf5c1d2f5c326e654acf8d9412ec 100644 (file)
--- a/docs/porting-guide.md
+++ b/docs/porting-guide.md
@@ -1473,9 +1473,8 @@ of interrupt depends upon the id value as follows.
     Return   : uint32_t
 
 This API returns the id of the highest priority pending interrupt at the
-platform IC. The IMF passes the id returned by this API to the registered
-handler for the pending interrupt if the `IMF_READ_INTERRUPT_ID` build time flag
-is set. INTR_ID_UNAVAILABLE is returned when there is no interrupt pending.
+platform IC. INTR_ID_UNAVAILABLE is returned when there is no interrupt
+pending.
 
 ARM standard platforms read the _Highest Priority Pending Interrupt
 Register_ (`GICC_HPPIR`) to determine the id of the pending interrupt. The id

diff --git a/docs/user-guide.md b/docs/user-guide.md
index a7a0c11f3d392c117028b16aa5f2f7af590ba0b8..b7b152e4f5bec71f7dbc3cca72a9e874ecc3b144 100644 (file)
--- a/docs/user-guide.md
+++ b/docs/user-guide.md
@@ -250,11 +250,6 @@ performed.
     is used to determine the number of valid slave interfaces available in the
     ARM CCI driver. Default is 400 (that is, CCI-400).
 
-*   `IMF_READ_INTERRUPT_ID`: Boolean flag used by the interrupt management
-    framework to enable passing of the interrupt id to its handler. The id is
-    read using a platform GIC API. `INTR_ID_UNAVAILABLE` is passed instead if
-    this option set to 0. Default is 0.
-
 *   `RESET_TO_BL31`: Enable BL3-1 entrypoint as the CPU reset vector instead
     of the BL1 entrypoint. It can take the value 0 (CPU reset to BL1
     entrypoint) or 1 (CPU reset to BL3-1 entrypoint).

diff --git a/services/spd/opteed/opteed_main.c b/services/spd/opteed/opteed_main.c
index fefc8a75e8f8ecb08ef02b94564c6f1679f042e2..7796fc4a51b05b1cd764a91f8dbd0333aa7dd50d 100644 (file)
--- a/services/spd/opteed/opteed_main.c
+++ b/services/spd/opteed/opteed_main.c
@@ -85,11 +85,6 @@ static uint64_t opteed_sel1_interrupt_handler(uint32_t id,
        /* Check the security state when the exception was generated */
        assert(get_interrupt_src_ss(flags) == NON_SECURE);
 
-#if IMF_READ_INTERRUPT_ID
-       /* Check the security status of the interrupt */
-       assert(plat_ic_get_interrupt_type(id) == INTR_TYPE_S_EL1);
-#endif
-
        /* Sanity check the pointer to this cpu's context */
        assert(handle == cm_get_context(NON_SECURE));
 

diff --git a/services/spd/tspd/tspd_main.c b/services/spd/tspd/tspd_main.c
index b8b67fadc85636b00b2fb336faddffb1d458ea1d..622316014990a24134c52994c69b80fcd28d9bdc 100644 (file)
--- a/services/spd/tspd/tspd_main.c
+++ b/services/spd/tspd/tspd_main.c
@@ -106,11 +106,6 @@ static uint64_t tspd_sel1_interrupt_handler(uint32_t id,
        /* Check the security state when the exception was generated */
        assert(get_interrupt_src_ss(flags) == NON_SECURE);
 
-#if IMF_READ_INTERRUPT_ID
-       /* Check the security status of the interrupt */
-       assert(plat_ic_get_interrupt_type(id) == INTR_TYPE_S_EL1);
-#endif
-
        /* Sanity check the pointer to this cpu's context */
        assert(handle == cm_get_context(NON_SECURE));
 
@@ -173,10 +168,6 @@ static uint64_t tspd_ns_interrupt_handler(uint32_t id,
        /* Check the security state when the exception was generated */
        assert(get_interrupt_src_ss(flags) == SECURE);
 
-#if IMF_READ_INTERRUPT_ID
-       /* Check the security status of the interrupt */
-       assert(plat_ic_get_interrupt_type(id) == INTR_TYPE_NS);
-#endif
        /*
         * Disable the routing of NS interrupts from secure world to EL3 while
         * interrupted on this core.